Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow reading walltime variables #289

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from
Draft

Conversation

mikekryjak
Copy link
Collaborator

@mikekryjak mikekryjak commented May 23, 2023

The walltime variables are defined here:

https://github.com/boutproject/BOUT-dev/blob/3d6cb4b7da6605e2d39828f07ece36c8d3886bcd/include/bout/monitor.hxx#L80-L110

They are slightly different in each processor which causes issues with xarray concatenation. For this reason, wall time stats have been dropped from xBOUT at load time so far.

This PR changes this. We read the proc 0 variables and drop the rest. This should be representative of the whole domain as the wtime load should be near identical for each processor anyway.

To keep the change simple, this also keeps three more variables from _BOUT_PER_PROC_VARIABLES which are not particularly useful (PE_XIND, PE_YIND and MYPE). Let me know if you think those should be removed.

Progress:

  • Implement the change and test that it works
  • Work through pytest to resolve test issues

- The variables are slightly different in each processor
- This causes issues with xarray concatenation.
- Read proc 0 variables and discard the rest
- (Wtime stats should be the nearly the same for all procs)
- Also saves other variables which are part of
_BOUT_PER_PROC_VARIABLES
@mikekryjak mikekryjak added the enhancement New feature or request label May 31, 2023
@mikekryjak mikekryjak marked this pull request as draft May 31, 2023 12:39
@mikekryjak mikekryjak self-assigned this May 31, 2023
@pep8speaks
Copy link

Hello @mikekryjak! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

Line 223:37: E701 multiple statements on one line (colon)

Line 475:5: E704 multiple statements on one line (def)
Line 625:5: E704 multiple statements on one line (def)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants